<fix>[core]: register IPv6 prefer config#4146
Open
MatheMatrix wants to merge 15 commits into
Open
Conversation
added 12 commits
May 27, 2026 12:43
Register management.server/prefer.ipv6 as a GlobalConfig so QueryGlobalConfig and runtime updates work. Keep the startup system property override and fall back to the legacy global property before GlobalConfig is available. Resolves: ZSTAC-85520 Change-Id: Ia59515ce9a9a29eeecb9b5ab7d83057823089884
Use bare IPv6 for ssh command targets. Keep bracketed IPv6 only for scp host:path syntax. Resolves: ZSTAC-85522 Change-Id: I0d655ccf654634edb109f9c8025a7b70dbf34da4
Use a callback URL in the same IP family as the virtual router management NIC. This keeps IPv4-only vrouter agents reachable when MN is dual-stack and its default management IP is IPv6. Resolves: ZSTAC-85527 Change-Id: Iaa630fc59d30c2675db7bbe47cb1b7c8d58bb023
Console proxy returned the MN IPv6 address to clients but selected the listen address from agentIp, which is 127.0.0.1 for the management-node agent. Use the client-facing proxy hostname to choose the wildcard listen address so IPv6 console URLs are reachable. Resolves: ZSTAC-85595 Change-Id: Ief18c9b847f0e0c050ce993a50244533614fd2b8
Fixes ZSTAC-85605 and ZSTAC-85612. SSH keeps raw IPv6 hosts while SCP brackets IPv6 paths. Ansible callback checker now passes -6 for IPv6 nc and nmap. Hotfix verified on 172.24.249.182. Resolves: ZSTAC-79206 Change-Id: Iaa7204e638335c7bf1496b2cd5e0314081e598cb
VXLAN CIDR system tags can contain IPv6 values with double colon. Split tag fields only outside token braces so patterned tag matching and token extraction keep IPv6 CIDRs intact. Resolves: ZSTAC-85618 Change-Id: Ie74a3ac89e1d728953bcaab74146d25e7a7e2edc
Remove the customer-facing management.server.prefer.ipv6 switch.\n\nUse explicit management.server.ip first. When it is not configured, keep IPv4 as the default and fall back to IPv6 only if no IPv4 is available.\n\nWaterfall CR: ZSTAC-79206 CR-001 Change-Id: I14e3e6b3fdad2e4e109d4f9c9f3f344356866762
Build KVM agent HTTP URLs through the IPv6-safe host formatter so live migration cleanup and migrate calls bracket IPv6 host addresses. Resolves: ZSTAC-85636 Change-Id: I0c9fa1eecf7d6eef778cbb4568123d7bd3a836ec
Allow patterned system tag values to contain IPv6 token text. Use dual-stack CIDR matching for migration network selection. Add focused management IPv6 regression assertions. Resolves: ZSTAC-85638 Change-Id: I32f3d1111aa3358fd26da6a3afb23111154f9058
Resolves: ZSTAC-79206 Change-Id: I485bade7bd875e2b492174f4c2e7e710fee1c0b3
Fixes ZSTAC-85690.\n\nAllow IPv6 ranges on System L3 networks for management network IPv6.\nAlso allow virtual router offerings to use IPv6 management L3. Change-Id: If46c0ab86c018f72d0138df6363689f85dbecaeb
Parse system tag delimiters only outside token braces when matching actual tag values. Resolves: ZSTAC-85618 Change-Id: I7d0a9cf3146a513e56ba0d6d3bff3685bc4716e0
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
Walkthrough该 PR 统一了 ZStack 对 IPv6 双栈网络的处理，移除了管理服务器 IPv6 偏好配置，重构了管理 IP 选择策略、回调 URL 体系，改进了网络工具与 URL/SSH/SCP 构造，并通过 token-aware 标签解析支持包含 IPv6 CIDR 的 patterned tag，涵盖 19 个文件的功能更新与 9 个测试用例的扩展。 ChangesIPv6 管理网络与回调 URL 体系
网络工具与 URL/SSH/SCP 统一处理
网络校验规则放宽
系统标签 Token-aware 解析
集成测试扩展
错误码常量扩展
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 warning, 2 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.42.3)utils/src/main/java/org/zstack/utils/clouderrorcode/CloudOperationsErrorCode.javaComment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsAgentUrl.java (1)
10-19: ⚡ Quick win建议在路径拼接时统一处理前后斜杠，避免出现
//。当前实现在
sb以/结尾且path也以/开头时会生成双斜杠，建议在appendPath中做一次归一化。♻️ 建议修改
private static void appendPath(StringBuilder sb, String path) { if (path == null || path.isEmpty()) { return; } - if (!path.startsWith("/")) { - sb.append("/"); - } - sb.append(path); + boolean sbEndsWithSlash = sb.length() > 0 && sb.charAt(sb.length() - 1) == '/'; + boolean pathStartsWithSlash = path.startsWith("/"); + + if (sbEndsWithSlash && pathStartsWithSlash) { + sb.append(path.substring(1)); + } else if (!sbEndsWithSlash && !pathStartsWithSlash) { + sb.append('/').append(path); + } else { + sb.append(path); + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsAgentUrl.java` around lines 10 - 19, appendPath currently can produce double slashes when sb ends with '/' and path starts with '/'; update the appendPath(StringBuilder sb, String path) to normalize boundaries by removing a trailing slash from sb or a leading slash from path before appending so exactly one '/' separates segments (handle null/empty path as before and ensure you still append a single '/' when needed). Use the existing sb and path parameters and make this normalization inside appendPath so callers need no changes.header/src/main/java/org/zstack/header/rest/RESTFacade.java (1)
91-91: ⚡ Quick win为新增接口方法补充 Javadoc。
RESTFacade是公共接口，Line 91 新增方法缺少方法级文档，后续调用约束不够清晰。建议补充参数与返回值语义说明。建议修改
+ /** + * Build callback URL with the given host name using current RESTFacade port/path. + * + * `@param` hostName callback host name or IP + * `@return` callback URL + */ String buildCallbackUrl(String hostName);As per coding guidelines "接口方法不应有多余的修饰符（例如
public），且必须配有有效的 Javadoc 注释。"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@header/src/main/java/org/zstack/header/rest/RESTFacade.java` at line 91, RESTFacade 接口中新加的方法 buildCallbackUrl(String hostName) 缺少方法级 Javadoc；请为方法 String buildCallbackUrl(String hostName) 添加完整的 Javadoc，明确说明参数 hostName 的含义（例如主机名或主机地址、是否允许为 null/空、预期格式）以及返回值语义（例如返回用于回调的完整 URL、何时可能返回 null 或抛出异常），并确保接口方法不带多余的修饰符以符合编码规范。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/src/main/java/org/zstack/utils/TagUtils.java`:
- Around line 49-50: The fast-return in isMatch that does "if
(fmt.indexOf(TAG_DELIMITER) == -1) return fmt.equals(tag);" wrongly treats
templates like "{token}" as non-matching; instead, change this branch to
consider template tokens: when TAG_DELIMITER is absent in fmt, return true if
fmt.equals(tag) OR fmt itself is a single token template that should match any
valid tag value (e.g., fmt.matches("\\{[^}]+}")) and tag is non-empty (or
matches the expected token pattern). Update isMatch to use these checks
(referencing isMatch, fmt, tag, and TAG_DELIMITER) so token-template strings are
treated as matching rather than always false.
---
Nitpick comments:
In `@header/src/main/java/org/zstack/header/rest/RESTFacade.java`:
- Line 91: RESTFacade 接口中新加的方法 buildCallbackUrl(String hostName) 缺少方法级
Javadoc；请为方法 String buildCallbackUrl(String hostName) 添加完整的 Javadoc，明确说明参数
hostName 的含义（例如主机名或主机地址、是否允许为 null/空、预期格式）以及返回值语义（例如返回用于回调的完整 URL、何时可能返回 null
或抛出异常），并确保接口方法不带多余的修饰符以符合编码规范。
In `@plugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsAgentUrl.java`:
- Around line 10-19: appendPath currently can produce double slashes when sb
ends with '/' and path starts with '/'; update the appendPath(StringBuilder sb,
String path) to normalize boundaries by removing a trailing slash from sb or a
leading slash from path before appending so exactly one '/' separates segments
(handle null/empty path as before and ensure you still append a single '/' when
needed). Use the existing sb and path parameters and make this normalization
inside appendPath so callers need no changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: cab5d952-05ef-4595-87b2-6204fdd2f192
📒 Files selected for processing (25)
console/src/main/java/org/zstack/console/ConsoleProxyBase.javacore/src/main/java/org/zstack/core/CoreGlobalProperty.javacore/src/main/java/org/zstack/core/Platform.javacore/src/main/java/org/zstack/core/ansible/CallBackNetworkChecker.javacore/src/main/java/org/zstack/core/rest/RESTFacadeImpl.javaheader/src/main/java/org/zstack/header/rest/RESTFacade.javanetwork/src/main/java/org/zstack/network/l3/L3NetworkApiInterceptor.javaplugin/ceph/src/main/java/org/zstack/storage/ceph/primary/CephPrimaryStorageBase.javaplugin/kvm/src/main/java/org/zstack/kvm/KVMHost.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouter.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterApiInterceptor.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterManager.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterManagerImpl.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/lifecycle/VirtualRouterDeployAgentFlow.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/vyos/VyosConnectFlow.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/MdsUri.javaplugin/zbs/src/main/java/org/zstack/storage/zbs/ZbsAgentUrl.javatest/src/test/groovy/org/zstack/test/integration/console/ConsoleProxyCase.groovytest/src/test/groovy/org/zstack/test/integration/core/ManagementNetworkIpv6Case.groovytest/src/test/groovy/org/zstack/test/integration/storage/primary/addon/zbs/ZbsPrimaryStorageCase.groovyutils/src/main/java/org/zstack/utils/TagUtils.javautils/src/main/java/org/zstack/utils/network/IPv6NetworkUtils.javautils/src/main/java/org/zstack/utils/network/NetworkUtils.javautils/src/main/java/org/zstack/utils/ssh/Ssh.javautils/src/main/java/org/zstack/utils/ssh/SshShell.java
💤 Files with no reviewable changes (3)
- network/src/main/java/org/zstack/network/l3/L3NetworkApiInterceptor.java
- core/src/main/java/org/zstack/core/CoreGlobalProperty.java
- plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterApiInterceptor.java
Comment on lines
+49
to
50
| if (fmt.indexOf(TAG_DELIMITER) == -1) { | ||
| return fmt.equals(tag); |
There was a problem hiding this comment.
isMatch 的无分隔符快速返回引入了 token 模板匹配回归。
Line 49-50 现在直接 fmt.equals(tag)，会把 "{token}" 这类合法模板错误判定为不匹配。
建议修复
+ if (tag == null) {
+ return false;
+ }
- if (fmt.indexOf(TAG_DELIMITER) == -1) {
+ if (fmt.indexOf(TAG_DELIMITER) == -1 && !isTokenField(fmt)) {
return fmt.equals(tag);
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@utils/src/main/java/org/zstack/utils/TagUtils.java` around lines 49 - 50, The
fast-return in isMatch that does "if (fmt.indexOf(TAG_DELIMITER) == -1) return
fmt.equals(tag);" wrongly treats templates like "{token}" as non-matching;
instead, change this branch to consider template tokens: when TAG_DELIMITER is
absent in fmt, return true if fmt.equals(tag) OR fmt itself is a single token
template that should match any valid tag value (e.g., fmt.matches("\\{[^}]+}"))
and tag is non-empty (or matches the expected token pattern). Update isMatch to
use these checks (referencing isMatch, fmt, tag, and TAG_DELIMITER) so
token-template strings are treated as matching rather than always false.
Format IPv6 vRouter agent URLs with brackets and select the MN callback source IP from the route to the agent.\n\nAlso expose local non-loopback IPs to appliance VM bootstrap selection so dual-stack MNs can provide an IPv6 managementNodeIp while keeping management.server.ip on IPv4. Resolves: ZSTAC-85692 Change-Id: If1612db0b39389084a0501fa1065b15cac1d9990
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/src/main/java/org/zstack/core/Platform.java`:
- Line 1068: The getManagementServerIps() method currently calls
getLocalNonLoopbackIps(), which widens its semantics and risks returning
non-management network IPs; change getManagementServerIps() to stop adding
getLocalNonLoopbackIps() directly and instead implement a focused fallback
helper (e.g., a new method or flag) that is only used by callers that explicitly
need any non-loopback addresses; update callers
VirtualRouterManagerImpl.selectManagementIpForAgent() and
ApplianceVmFacadeImpl.putManagementNodeBootstrapParams() to invoke the new
explicit fallback when appropriate (and keep existing behavior of
getManagementServerIp4/6 and vrManagementCidrs filtering intact), ensuring any
fallback result is further filtered by management CIDRs or explicit
management-interface selection before being returned.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: http://open.zstack.ai:20001/code-reviews/zstack-cloud.yaml (via .coderabbit.yaml)
Review profile: CHILL
Plan: Pro
Run ID: 5457217c-23b4-4f06-8a71-7a52a0709b60
📒 Files selected for processing (2)
core/src/main/java/org/zstack/core/Platform.javaplugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterManagerImpl.java
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/virtualRouterProvider/src/main/java/org/zstack/network/service/virtualrouter/VirtualRouterManagerImpl.java
added 2 commits
May 30, 2026 13:26
Merge latest feature branch and keep IPv6 vRouter callback URL behavior. Resolves: ZSTAC-79206 Change-Id: I67d1c41833cc649b12239223f2449bf297fadad7
Keep getManagementServerIps narrow and use explicit local fallback only for appliance VM bootstrap CIDR selection. Resolves: ZSTAC-79206 Change-Id: Ib90e405fcef57b0dcbe03703ddcf2fe92682745b
| public static String selectManagementServerIp(Collection<InetAddress> addresses, boolean preferIpv6) { | ||
| public static List<String> getManagementServerIpsWithLocalFallback() { | ||
| LinkedHashSet<String> ips = new LinkedHashSet<>(getManagementServerIps()); | ||
| ips.addAll(getLocalNonLoopbackIps()); |
Collaborator
There was a problem hiding this comment.
Comment from shixin.ruan:
有效，已按最小范围修复。缺陷类型：API 语义/多网卡回调候选扩大。根因是 getManagementServerIps() 被扩展为包含所有本机非回环地址，确实会破坏旧 API 的“管理地址列表”契约。修改：9c58c88c32 中恢复 getManagementServerIps() 只返回 management.server.ip / ip4 / ip6；新增 getManagementServerIpsWithLocalFallback()，仅在 ApplianceVmFacadeImpl bootstrap 且后续会按 vRouter management CIDR 过滤的路径使用。VirtualRouterManagerImpl 仍优先使用 Platform.getRouteSourceIp(agentIp)，fallback 回窄化后的管理地址列表。验证：mvn -pl utils -DskipTests install；mvn -pl utils,header,core,plugin/applianceVm,plugin/virtualRouterProvider -DskipTests compile；zstack/test 下 mvn -DskipTests test-compile 均通过。剩余风险：本次不引入按网卡名选择管理口的新策略，只收窄旧 API 语义并保留 bootstrap CIDR 过滤 fallback。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Register management.server/prefer.ipv6 as a GlobalConfig so
QueryGlobalConfig and runtime updates work. Keep the startup system
property override and fall back to the legacy global property before
GlobalConfig is available.
Resolves: ZSTAC-85520
Change-Id: Ia59515ce9a9a29eeecb9b5ab7d83057823089884
sync from gitlab !10050